Make the "environment" parameter to @JavaMethod initializers optional #102
Conversation
…ly-typed and default to nil
There was a problem hiding this comment.
Looks ok to me, seems that's what @DougGregor had in mind -- leaving to him to approve the javakit bits.
Module wise I'm not sure if the separate VM module gained us anything, either seems fine.
| return [ | ||
| """ | ||
| self = \(raw: tryKeyword) Self.dynamicJavaNewObject(in: environment\(raw: arguments)) | ||
| let _environment = environment == nil ? \(raw: tryKeyword) JavaVirtualMachine.shared().environment() : environment! |
There was a problem hiding this comment.
maybe this would be nicer, without resorting to the !
| let _environment = environment == nil ? \(raw: tryKeyword) JavaVirtualMachine.shared().environment() : environment! | |
| let environment = | |
| if let environment { | |
| environment | |
| } else { | |
| \(raw: tryKeyword) JavaVirtualMachine.shared().environment() | |
| } |
There was a problem hiding this comment.
Ah, I had the same comment earlier ;)
DougGregor
left a comment
DougGregor
left a comment
There was a problem hiding this comment.
I would like to remove the unnecessary force-unwraps, but otherwise this looks good to me and thank you!
| public init(_ enumValue: \(raw: name), environment: JNIEnvironment) { | ||
| let classObj = try! JavaClass<Self>(in: environment) | ||
| public init(_ enumValue: \(raw: name), environment: JNIEnvironment? = nil) { | ||
| let _environment = environment == nil ? try! JavaVirtualMachine.shared().environment() : environment! |
There was a problem hiding this comment.
I'd rather do this without the force-unwraps, even if it's more verbose:
let _environment = if let environment {
environment
} else {
try! JavaVirtualMachine.shared().environment()
}| return [ | ||
| """ | ||
| self = \(raw: tryKeyword) Self.dynamicJavaNewObject(in: environment\(raw: arguments)) | ||
| let _environment = environment == nil ? \(raw: tryKeyword) JavaVirtualMachine.shared().environment() : environment! |
There was a problem hiding this comment.
Ah, I had the same comment earlier ;)
The VM module really didn't seem to be buying us anything, so I'm fine with collapsing it into JavaKit. |
…ed on PR conmments
|
LGTM, thanks a lot 👍 |
4 participants
Closes #70
Make the "environment" parameter to @JavaMethod initializers optionally-typed and default to nil